Skip to content

bpo-37219: Remove errorneous optimization for differencing an empty set#13965

Merged
rhettinger merged 2 commits into
python:masterfrom
rhettinger:set_diff_bug
Jun 11, 2019
Merged

bpo-37219: Remove errorneous optimization for differencing an empty set#13965
rhettinger merged 2 commits into
python:masterfrom
rhettinger:set_diff_bug

Conversation

@rhettinger

@rhettinger rhettinger commented Jun 11, 2019

Copy link
Copy Markdown
Contributor

@tirkarthi tirkarthi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this case have a test for TypeError since this looks like reverting 4103e4dfbce .

@serhiy-storchaka

Copy link
Copy Markdown
Member

LGTM, but would be nice to add a test.

We can check the size of the set in the loop. This would give the nefit not only for empty set, but in the case of substracting the larger set from the smaller set. But this is new feature and different issue.

@rhettinger

Copy link
Copy Markdown
Contributor Author

FYI, I don't want to put a size check inside the loop.

@tirkarthi tirkarthi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@serhiy-storchaka

Copy link
Copy Markdown
Member

Please add also a test for an iterable of unhasables, e.g. [[]]. It will trigger different error.

@rhettinger

Copy link
Copy Markdown
Contributor Author

I don't know what you're talking about. That gives a TypeError just like the existing test.

@rhettinger
rhettinger merged commit 1f11cf9 into python:master Jun 11, 2019
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@miss-islington

Copy link
Copy Markdown
Contributor

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

@miss-islington

Copy link
Copy Markdown
Contributor

Sorry, @rhettinger, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1f11cf9521114447b3e32e2ac88f075ffaa37555 3.7

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington

Copy link
Copy Markdown
Contributor

Sorry @rhettinger, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 1f11cf9521114447b3e32e2ac88f075ffaa37555 3.8

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot

Copy link
Copy Markdown

GH-13967 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 11, 2019
pythonGH-13965)

(cherry picked from commit 1f11cf9)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
@bedevere-bot

Copy link
Copy Markdown

GH-13968 is a backport of this pull request to the 3.7 branch.

rhettinger added a commit that referenced this pull request Jun 11, 2019
GH-13965) (GH-13967)

(cherry picked from commit 1f11cf9)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
@serhiy-storchaka

Copy link
Copy Markdown
Member

These errors are different.

>>> set().difference(123)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'int' object is not iterable
>>> set().difference([[]])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'list'

If add the check for the set size in the loop, the former will left, but the latter will disappear. Someone can add such optimization and miss the behavior change. See https://bugs.python.org/issue28071#msg306973.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants